Skip to content

Order scopes on save, and optimize the happy path for access token read - #644

Merged
Ray Luo (rayluo) merged 3 commits into
devfrom
order-scopes
Jan 9, 2024
Merged

Order scopes on save, and optimize the happy path for access token read#644
Ray Luo (rayluo) merged 3 commits into
devfrom
order-scopes

Conversation

@rayluo

@rayluo Ray Luo (rayluo) commented Jan 3, 2024

Copy link
Copy Markdown
Contributor

This PR currently contains 2 commits.

  1. The first commit orders scopes on save, which is similar to Order scopes on save to avoid access token duplication in the cache microsoft-authentication-library-for-dotnet#4479. Existing unit tests need adjustment to match the changed scope order.
  2. The second commit optimizes the happy path of access token read, which was the idea mentioned in this conversation. Basically, when searching for an access token, this PR uses the token key to attempt an O(1) search, and falls back to O(n) search when necessary.
    • No other unit test adjustment is needed, meaning this change is backward compatible.
    • Existing benchmark test shows ~40% time reduction (or 1.6x speed improvement) on "token cache hit" code path, which is also visible in diagram No. 2 and No. 4 in this benchmark page. (Note that the performance gain can be much higher than 1.6x if each tenant has more tokens; it is actually an O(1) vs O(n) improvement.)

Comment thread msal/token_cache.py Outdated
@bgavrilMS

Copy link
Copy Markdown
Member

Nice perf result. CC Peter (@pmaytak) as the same strategy could be done in .NET

Comment thread msal/token_cache.py Outdated
@rayluo
Ray Luo (rayluo) merged commit 804d529 into dev Jan 9, 2024
@rayluo
Ray Luo (rayluo) deleted the order-scopes branch January 9, 2024 00:44
Comment thread msal/application.py
@rayluo

Copy link
Copy Markdown
Contributor Author

If you can take initiative to resolve that will be highly appreciated.

@Moazzem-Hossain-pixel , you are commenting on a topic that has already been implemented. There are some follow-ups in their own github issues linked above, and we are addressing them respectively. But the main change in the current topic is already stable. And our downstream partner - Azure CLI - has been unblocked by AzureAD/microsoft-authentication-extensions-for-python#127

If you are blocked by any specific issue, please add your comment into those linked issues, or even create a new issue to elaborate. Meanwhile, I'm locking this issue here.

@AzureAD Azure Active Directory (AzureAD) locked as resolved and limited conversation to collaborators May 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants